Woof, Woof!

Thank you @3rdvision for submitting the "#357 Debouncer for setting the initials" pull request 😎.

Please do not forget to review our internal guidelines:

• Review the code in the PR using a peer 🧑‍🤝‍🧑
• Make sure all the linting process has been executed an passed 👌
• Make sure unit tests are passing ✅
• Ensure that proper code standards are met 📋
• Fill in the PR's summary (issue, dependencies, decisions and GIF)
• Double-check the diff to make sure it's the absolute best solution you can come up with

Engaging in the development process in the best possible way helps it being efficient and fast.

Your friend,
Tobias (Platforme's mascot)

Woof, Woof!

Oops! @3rdvision you may have to fix some errors:

• Please try to solve the conflicts and then re-assign PR. ❗️

Please address these issues to continue with the Pull Request merge process.

Your friend,
Tobias (Platforme's mascot)

Just to check, did you explore a possible solution where a cancel flag is used to stop the update promise before the existence of loadedCallback?

I did made an implementation of it but it didn't offer much performance difference. The problem is update will await any previous updates here after it cancels pending updates here and there wasn't a significant performance improvement, maybe about from 6.2s to 5.2s while using the debouncer it's about 6.2s to 3.1s.

It's also difficult to distinguish set initials update. In some places we have reason specifying set initials but because we also trigger initials_extra to notify the client that the initials changed after setInitialsExtra, we have a bind for initials_extra and in other places which will do a setInitials later without including params.reason="set initials_extra" and even thought we could refactor to always track it, from what I've tried, that experience didn't hold significant performance improvements versus this one.

Just to check, did you explore a possible solution where a cancel flag is used to stop the update promise before the existence of loadedCallback?

I did made an implementation of it but it didn't offer much performance difference. The problem is update will await any previous updates here after it cancels pending updates here and there wasn't a significant performance improvement, maybe about from 6.2s to 5.2s while using the debouncer it's about 6.2s to 3.1s.

It's also difficult to distinguish set initials update. In some places we have reason specifying set initials but because we also trigger initials_extra to notify the client that the initials changed after setInitialsExtra, we have a bind for initials_extra and in other places which will do a setInitials later without including params.reason="set initials_extra" and even thought we could refactor to always track it, from what I've tried, that experience didn't hold significant performance improvements versus this one.

What I was sugesting was end the updatePromise earlier on cancel, since if the update process did yet not reach the new Promise..... that is saved as loadedCallback when the cancel is called, the whole update is made. With the approach I mentioned, this does not happen. For example, when writting a whole word, only the last request is completed (with the whole word), instead of the first and then the last.

Example inside _update closure in update:

...
// verifies if the model currently loaded in the Ripe Instance can
// render the frame to be display and if that's not the case "ignores"
// the current request for update
if (frame && !this.owner.hasFrame(frame)) {
    this.trigger("not_loaded");
    return false;
}

if (this.cancelFlag) return;

// builds the URL of the image using the frame hacking approach
// this should provide us with the new values
const url = this.owner._getImageURL({
    frame: frame,
    ...
});
...

The cancelFlag is set as true in the cancel method and as false after the cancel call in update. If the previous update has not yet reached the assignment of this._loadedCallback, the _update method would return instead of executing until the end.

Closing this in favor of a more performant approach #366